fix(caldav): Expand recurring events for principal calendar search#60766
fix(caldav): Expand recurring events for principal calendar search#60766kesselb wants to merge 10 commits into
Conversation
kesselb
requested review from
ChristophWurst,
SebastianKrupinski and
tcitworld
as code owners
kesselb
requested review from
CarlSchwan,
artonge,
leftybournes and
salmart-dev
and removed request for
a team
|
/backport to stable34 |
|
/backport to stable33 |
|
/backport to stable32 |
| // Expand recurrences if an explicit time range is requested | ||
| if ($calendarData instanceof VCalendar && isset($start, $end)) { | ||
| $calendarData = $calendarData->expand($start, $end); | ||
| } | ||
|
|
There was a problem hiding this comment.
Hey,
So I understand the need to have the expanded events, but I think this is the wrong place to implement this. The back end is a storage interface and it should handle only the storing and retrieval of data, not the manipulation of the data. This should be done by the caller, as the expansion is specifically needed by the event search and not all other callers.
Also, the expand function is highly inefficient, there can be hundreds or event thousands of event returned from the original search, this now forces the creation of event more new instances of those events before returning.
I would recommend doing this in the search provider and using the EventReader, which is way more efficient at finding the the next occurrence of a recurring event.
https://github.com/nextcloud/server/blob/master/apps/dav/lib/CalDAV/EventReader.php
There was a problem hiding this comment.
The first iteration did the expansion in the EventSearchProvider, so that works too. Christoph and I discussed it recently. CalDavBackend.search (the one for a single calendar) also does the expansion, and adjusting searchPrincipalUri ensures that both methods behave more similarly than before.
That said, both approaches make sense to me. Can you and @ChristophWurst figure that out? I don't have a strong preference either way.
|
I approved by mistake! It was meant to be a comment! |
Assisted-by: Claude:claude-sonnet-4-6 Assisted-by: OpenCode:github-copilot/gpt-5.4 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb
force-pushed
the
bug/noid/expand-unified-search
branch
from
5c8045f to
cf432ac
Compare
Summary
Expand recurring events when searching for events through unified search.
STR:
Main: Event 2024-05-26 is shown
Here: Event 2026-05-26 is shown
Checklist
3. to review, feature component)stable32)AI (if applicable)